-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Accept case insensitive values in boolean settings #10663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Accept case insensitive values in boolean settings #10663
Conversation
@blueorangutan package |
@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #10663 +/- ##
=========================================
Coverage 16.14% 16.15%
- Complexity 13253 13273 +20
=========================================
Files 5656 5656
Lines 497893 497888 -5
Branches 60374 60373 -1
=========================================
+ Hits 80405 80442 +37
+ Misses 408529 408488 -41
+ Partials 8959 8958 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12955 |
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Show resolved
Hide resolved
@blueorangutan package |
@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13279 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm and unit tests provided. never the less I think this is such a cross cutting concern that it will need extensive testing.
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13225)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the changes in my local environment and everything worked as expected for booleans, strings, integers, and floats.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@bernardodemarco could you fix the merge conflicts here? |
12f295d
to
466c76d
Compare
Sure, done! |
@blueorangutan package |
@DaanHoogland @weizhouapache @Pearl1594, can we run the CI again for this one? |
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13612 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13479)
|
Description
Currently, when updating boolean settings, the Management Server performs a case sensitive validation of the inserted values. Thus, only the
true
andfalse
values are accepted.This PR proposes to extend this validation to encompass case insensitive boolean values. Therefore, values such as
True
andFALSE
will be accepted and will be properly normalized before being persisted in the database.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
quota.enable.service
global setting toFaLsE
.updateConfiguration
API executionPersisted configuration value in the database
TRUe
:updateConfiguration
API executionPersisted configuration value in the database